[_795] allow options to be accessed as attrs of metadata obj #796
Conversation
alanking
left a comment
alanking
left a comment
There was a problem hiding this comment.
Please address or explicitly ignore Ruff as you see fit.
irods/meta.py
Outdated
| if n in _default_MetadataManager_opts: | ||
| return self._manager._opts[n] | ||
| raise AttributeError |
There was a problem hiding this comment.
The use of _default_MetadataManager_opts is a little confusing to me. Does this mean that we cannot access non-default options?
Does something like this improve the situation, or make it worse?
| if n in _default_MetadataManager_opts: | |
| return self._manager._opts[n] | |
| raise AttributeError | |
| if (attr := self._manager._opts.get(n)) is None: | |
| raise AttributeError(f"Attribute [{n}] not found") | |
| return attr |
There was a problem hiding this comment.
I tried it that way and got a never-ending recursion, unfortunately : (
There was a problem hiding this comment.
Hmm, I wonder if it has to do with the fact that iRODSMeta is the iRODSMeta_type for _opts...
Does this work?
| if n in _default_MetadataManager_opts: | |
| return self._manager._opts[n] | |
| raise AttributeError | |
| if n in self._manager._opts: | |
| return self._manager._opts[n] | |
| raise AttributeError |
I'm mostly trying to see if we can avoid having to use _default_MetadataManager_opts.
There was a problem hiding this comment.
I think that's how I originally tried it. Or, similar. When these infinite loopings happen, it's a pain to figure out why. I'll give it another go.
There was a problem hiding this comment.
(py3) daniel@yuggoth:~/python-irodsclient$ python try.py
Traceback (most recent call last):
File "/home/daniel/python-irodsclient/try.py", line 4, in <module>
d.metadata(admin=True).admin
^^^^^^^^^^^^^^^^^^^^^^
File "/home/daniel/python-irodsclient/irods/meta.py", line 151, in __call__
x = copy.copy(self)
^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/copy.py", line 97, in copy
return _reconstruct(x, None, *rv)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/copy.py", line 260, in _reconstruct
if hasattr(y, '__setstate__'):
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/daniel/python-irodsclient/irods/meta.py", line 139, in __getattr__
if n in self._manager._opts:
^^^^^^^^^^^^^
File "/home/daniel/python-irodsclient/irods/meta.py", line 139, in __getattr__
if n in self._manager._opts:
^^^^^^^^^^^^^
File "/home/daniel/python-irodsclient/irods/meta.py", line 139, in __getattr__
if n in self._manager._opts:
^^^^^^^^^^^^^
[Previous line repeated 993 more times]
RecursionError: maximum recursion depth exceededThere was a problem hiding this comment.
The above dump is the result. That's why I changed the code to be what it currently is....
I don't know why the one self._manager reference is ok but the other isn't.
It's a mystery....
There was a problem hiding this comment.
Well, that stinks. Okay, please leave a comment regarding this situation above the usage of _default_MetadataManager_opts here and above the definition of _default_MetadataManager_opts in metadata_manager.py so that we understand why it is being done this way.
There was a problem hiding this comment.
The problem appears to be that the new object m has not had _opts set on it yet. Still, why it's a problem when using in but not in the return statement is strange.
|
|
||
|
|
||
| class iRODSMetaCollection: | ||
| def __getattr__(self,n): |
There was a problem hiding this comment.
Consider changing n to name or some other more descriptive parameter name.
irods/manager/metadata_manager.py
Outdated
| _default_MetadataManager_opts = { | ||
| 'admin':False, | ||
| 'timestamps':False, | ||
| 'iRODSMeta_type':iRODSMeta, | ||
| 'reload':True | ||
| } |
There was a problem hiding this comment.
What does this buy us over just... adding the new option to self._opts?
There was a problem hiding this comment.
Somehow doing it this way avoided a bottomless recursion. I do not know why....
There was a problem hiding this comment.
Please leave a comment above this explaining why it is necessary, and we can resolve this.
There was a problem hiding this comment.
I know you said you don't know why, but at least describe what we are trying to prevent by doing it this way even if we don't know why the situation is happening.
There was a problem hiding this comment.
OK.
Found a link that might provide insight.
https://nedbatchelder.com/blog/201010/surprising_getattr_recursion
I'm still going to try tracking it down because the copy.copy() has already happened in this case, so it... shouldn't be happening but is.
2 participants
No description provided.